Mode A coupon-reassignment automation — M1+M2 foundation [DRAFT: M3+M4 to follow; needs split-source decision] - #254
Closed
gyorgybalazsi wants to merge 7 commits into
Closed
Conversation
…1.19 (devnet-matched)
Adds AssignRewardBeneficiaries, a GovernableAction that lets governance assign beneficiaries to CIP-104 RewardCouponV2 coupons via the RewardCoupon_AssignBeneficiaries choice, so coupons can be minted before they expire. Ships with its own test package (governance-rewards-assign-test) depending on amulet 0.1.19 (for RewardCouponV2) alongside governance-rewards-v1, since the amulet 0.1.17/0.1.19 version split anticipated in the plan did not in fact conflict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dation Adds the RewardBeneficiary struct and AssignRewardBeneficiaries variant (primary_coupon + additional_coupons + new_beneficiaries, mirroring the DAML template so "at least one coupon" is a type guarantee) plus a validate() arm enforcing non-empty beneficiaries, <= 20 entries, each percentage in (0.0, 1.0], and an exact sum of 1.0. Backend-only (Task 2 of 4); the serializer arm lands in Task 3.
…Strings, not CantonId primary_coupon/additional_coupons hold ledger contract-ids, not parties. CantonId::parse requires a prefix::hexNamespace shape and rejects bare contract-id strings, so a real coupon cid could never be submitted through the JSON API. Type them as String like every other cid-bearing field in this file; RewardBeneficiary.beneficiary stays CantonId since it is genuinely a party.
Design (L1/L2/L3 actor model; Mode A assign-&-self-mint cranker) and the M1/M2 implementation plan, so reviewers can see the full scope. M3 (auto-confirm engine + cranker) and M4 (devnet IT) land in the same PR once the split-source question in the PR description is settled.
There was a problem hiding this comment.
Pull request overview
Adds the M1+M2 “foundation” for Mode A CIP-104 reward coupon beneficiary assignment: a new DAML governance action wrapping Splice’s RewardCoupon_AssignBeneficiaries, plus the corresponding Rust proposal plumbing (types, validation, and serialization), with dedicated DAML tests and supporting design/plan docs.
Changes:
- Introduces
AssignRewardBeneficiariesas aGovernableActioningovernance-rewards, exercisingRewardCoupon_AssignBeneficiarieswith emptyExtraArgs. - Adds a new DAML test package (
governance-rewards-assign-test) to validate the action behavior againstRewardCouponV2(incl. negative cases). - Extends the Rust backend with
ProposalType::AssignRewardBeneficiaries, boundary validation, andaction_serializermapping + shape test.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/reward-cranker/m1-m2-implementation-plan.md | Implementation plan for M1+M2 (DAML action + Rust plumbing); contains a few doc/code mismatches around emptyExtraArgs sourcing. |
| docs/reward-cranker/design.md | Design doc for the overall Mode A reward cranker (M1–M4 plan and architecture). |
| daml/multi-package.yaml | Registers the new governance-rewards-assign-test package. |
| daml/governance-rewards/daml/Governance/Rewards/AssignRewardBeneficiaries.daml | Adds the on-ledger AssignRewardBeneficiaries action that exercises Splice reward assignment. |
| daml/governance-rewards/daml.yaml | Adds the reward-assignment (and token-metadata) DARs as data-dependencies for the new action. |
| daml/governance-rewards-assign-test/daml/Governance/Rewards/TestAssignRewardBeneficiaries.daml | Adds script tests for happy path + failure cases for assignment. |
| daml/governance-rewards-assign-test/daml/Governance/Rewards/AssignTestUtils.daml | Adds local test utilities to drive propose→confirm→execute flows without pulling in wallet-bearing test closures. |
| daml/governance-rewards-assign-test/daml.yaml | Defines the new test package and its DAR dependencies (incl. amulet 0.1.19). |
| crates/decman/src/server/types.rs | Adds RewardBeneficiary, ProposalType::AssignRewardBeneficiaries, and backend boundary validation + unit tests. |
| crates/decman/src/server/action_serializer.rs | Adds serialization logic and a shape test for AssignRewardBeneficiaries proposal creation args. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **Package placement:** the new DAML lives in the `governance-rewards` package (`daml/governance-rewards/`) created by PR #248. That PR is a **prerequisite** — it also added `ProposalPackage::GovernanceRewards`, the `governance_rewards` `PackageConfig` field + default `#governance-rewards-v1`, and the `propose_action` handler arm for `GovernanceRewards`. **Do not re-add those; reuse them.** | ||
| - **Actor-model precision (spec §2):** a party id is never an actor. `governanceParty == provider` (spec §4.1, verified for CBTC). The governed execute carries the decparty's own authority — no extra delegation contract. | ||
| - **Splice assign semantics (verified in `splice/daml/splice-api-reward-assignment-v1/daml/Splice/Api/RewardAssignmentV1.daml`):** `RewardBeneficiary { beneficiary : Party, percentage : Decimal }`; `RewardCoupon_AssignBeneficiaries` takes `additionalCoupons : [ContractId RewardCoupon]`, `newBeneficiaries : [RewardBeneficiary]`, `extraArgs : ExtraArgs`; percentages must be in (0.0, 1.0], sum to 1.0, ≤ `maxNumNewBeneficiaries` (≤20), and every target coupon MUST have no beneficiary yet. `emptyExtraArgs` is exported from `Splice.Api.Token.MetadataV1`. |
| - Modify: `daml/multi-package.yaml` (register `governance-rewards-assign-test`). | ||
|
|
||
| **Interfaces:** | ||
| - Consumes: `Governance.Action (GovernableAction, GovernableActionView(..))`; `Splice.Api.RewardAssignmentV1`; `Splice.Api.Token.MetadataV1 (emptyExtraArgs)`; the copied `AssignTestUtils`. |
Comment on lines
+185
to
+190
| import Splice.Api.RewardAssignmentV1 | ||
| (RewardCoupon, RewardBeneficiary, RewardCoupon_AssignBeneficiaries(..)) | ||
| import Splice.Api.Token.MetadataV1 (emptyExtraArgs) | ||
|
|
||
| import Governance.Action | ||
|
|
|
|
||
| **Why a separate test package (and the one build risk):** PR #248's `governance-rewards-test` depends on `splice-wallet-0.1.18` (→ amulet `0.1.17`). This test needs amulet `0.1.19` (for `RewardCouponV2`). To keep the two amulet versions apart, the assign tests get their own package that depends on amulet `0.1.19` (not wallet). **Risk:** this test package must also depend on `governance-rewards-v1` (which contains `SetupMintingDelegation` and so transitively pulls `splice-wallet-0.1.18` → amulet `0.1.17`). If `dpm build` reports an amulet-version conflict between `0.1.17` and `0.1.19`, resolve it by **moving `AssignRewardBeneficiaries` into its own wallet-free package** (e.g. `governance-reward-assign-v1`, depending only on `governance-action-v1` + `reward-assignment-v1`) so nothing drags amulet `0.1.17` into the test closure — and update Task 2/3's package references accordingly. If neither coexistence nor the split resolves it, report BLOCKED with the exact `dpm` error. | ||
|
|
||
| - [ ] **Step 0: Scaffold the test package.** Create `daml/governance-rewards-assign-test/daml.yaml` (SDK `3.4.11`, matching PR #248's test package) with `data-dependencies`: `../governance-action-v1/.daml/dist/governance-action-v1-0.1.0.dar`, `../governance-core/.daml/dist/governance-core-v1-0.1.0.dar`, `../governance-rewards/.daml/dist/governance-rewards-v1-0.1.0.dar`, `../dars/splice-amulet-0.1.19.dar`, `../dars/splice-api-reward-assignment-v1-1.0.0.dar`, `../dars/testlib-0.1.0.dar` (copy the exact dep paths/build-options from `daml/governance-rewards-test/daml.yaml`, swapping `splice-wallet-0.1.18.dar` → `splice-amulet-0.1.19.dar`). Copy the four helpers into `AssignTestUtils.daml`. Register the package in `daml/multi-package.yaml`. |
gyorgybalazsi
changed the title
Contributor
Author
|
Superseded by #255. This PR was auto-closed when the head branch was renamed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
❓ Question for Robert (this is why the PR is up now)
The automation (M3) assigns a decparty's
RewardCouponV2coupons to their governance-configured beneficiary split (for CBTC today, live on devnet: operator 20% / cbtc-beneficiary 80%). I need to pin where the automation reads that split from on-ledger, because it shapes M3's core:Q1 — split source. Two options:
InstrumentConfiguration.providerAppRewardBeneficiaries+AppRewardConfiguration.operatorAppRewardBeneficiaryand re-runs thegetBeneficiariescomposition in Rust; or[RewardBeneficiary](beneficiary + percentage) that the automation reads directly.(b) avoids re-implementing
getBeneficiariesin DecMan and gives us a stable interface; (a) works against existing on-ledger contracts today but re-implements the composition and reads cross-package utility-registry state. Which should M3 target?Q2 — operator cut on the V2 path. Does the operator 20% cut apply on the V2 assign path (
RewardCoupon_AssignBeneficiaries), or only on the legacy activity-marker path (wheregetBeneficiariesruns)? I.e. under V2, does the automation assign[operator 0.2, cbtc-beneficiary 0.8], or just theproviderAppRewardBeneficiaries?Q3 (minor) — mode selector. Your shared "mode A/B per decparty" template isn't on
mainyet. M3 will gate the automation on a per-decparty "Mode A" flag; if your template will carry that selector, the automation can read the mode from there — otherwise I'll add a small per-decparty config. Which do you expect?What's in this PR so far (M1 + M2 — the foundation the automation drives)
An operator (and later the automation) can now assign reward-coupon beneficiaries through the normal propose → confirm → execute governance flow.
daml/governance-rewards/): newAssignRewardBeneficiariesGovernableActionwhoseexecuteImplexercises splice'sRewardCoupon_AssignBeneficiaries.governanceParty == provider, so the governed execute carries the decparty's own authority — no extra delegation contract. Tests (own package, to keep amulet0.1.19separate from the wallet-0.1.18path): happy-path (asserts per-beneficiary coupons created + originals archived) + two negatives.ProposalType::AssignRewardBeneficiaries+RewardBeneficiary, a boundaryvalidate()(non-empty, percentages in (0,1] summing to exactly 1.0, ≤20), and theaction_serializermapping. Reuses PR feat(governance-rewards): SetupMintingDelegation plugin for CIP-104 reward collection #248'sProposalPackage::GovernanceRewardswiring.splice-amulet-0.1.19+splice-api-reward-assignment-v1-1.0.0— package-ids verified byte-identical to what devnet's DSO issues coupons under, so the assign applies to live coupons. The main package depends only on the reward-assignment interface (no amulet), so the repo amulet is not bumped.de76b82): coupon fields are contract-idStrings, notCantonId(aCantonIdcan't hold an opaque contract-id) — caught after review.Design + plan for full context:
docs/coupon-reassignment/design.mdanddocs/coupon-reassignment/m1-m2-implementation-plan.md.Verified: DAML 3/3 scripts pass,
dpm build --allgreen,cargo test -p decmangreen (fmt + clippy-D warningsclean). A whole-branch review found no Critical/Important issues.Still to come in this PR (M3 + M4)
cbtc-networkcoupons.🤖 Generated with Claude Code